-
Notifications
You must be signed in to change notification settings - Fork 5
Feat/emover #468
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feat/emover #468
Conversation
WalkthroughIntroduces a comprehensive evault migration platform with a new Emover API backend and frontend, enabling users to migrate data between evault instances. Adds database copy functionality to evault-core, registry vault management endpoints, and supporting infrastructure including multi-database initialization and environment configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / eID Wallet
participant Frontend as Emover Frontend
participant API as Emover API
participant Registry as Registry
participant Provisioner as Provisioner
participant OldEVault as Old eVault
participant NewEVault as New eVault
User->>Frontend: Login (scan QR)
Frontend->>API: GET /api/auth/offer (SSE session)
API-->>Frontend: QR data + session ID
User->>OldEVault: Sign login challenge
OldEVault->>API: POST /api/auth/callback (signed)
API->>Registry: Verify token/user
Registry-->>API: User validated
API->>Frontend: SSE: Login complete (token)
Frontend->>Frontend: Store auth + redirect to dashboard
Frontend->>API: GET /api/evault/current
API->>Registry: Resolve current eVault for eName
Registry-->>API: Current eVault info
API-->>Frontend: Display eVault + provisioners
User->>Frontend: Initiate migration (select provisioner)
Frontend->>API: POST /api/migration/initiate
API->>Registry: Resolve old eVault data
API->>Database: Create Migration record (INITIATED)
API-->>Frontend: Return migrationId
Frontend->>API: POST /api/migration/sign {migrationId}
API->>Database: Create signing session
API-->>Frontend: Return QR (signing session)
User->>OldEVault: Sign migration challenge (via QR)
OldEVault->>API: POST /api/migration/callback {signature}
API->>API: Validate signature
API->>Frontend: SSE: Signing confirmed
Frontend->>Frontend: Start provisioning poll
API->>Provisioner: POST /provision (old key, entropy, namespace)
Provisioner->>NewEVault: Initialize new eVault
Provisioner-->>API: New eVault provisioned {w3id, uri, evaultId}
API->>Database: Update Migration (PROVISIONING → COPYING)
API->>OldEVault: POST /emover {eName, targetNeo4j}
OldEVault->>OldEVault: Copy meta-envelopes to new Neo4j
OldEVault-->>API: Copy complete {count}
API->>Database: Update Migration (COPYING → VERIFYING)
API->>Registry: Update mapping {eName → newEvaultId}
API->>Registry: Verify new eVault registered
API->>Database: Update Migration (VERIFYING → MARKING_ACTIVE)
API->>Database: Mark Migration (COMPLETED)
API->>Frontend: SSE: Migration complete
Frontend->>Frontend: Display success + redirect
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (17)
platforms/emover/tsconfig.json-1-28 (1)
1-28: Emover tsconfig should extend from the shared@metastate/typescript-config/nextjspackage for consistency with project standards.The configuration works correctly with TypeScript 5.8.2 (which fully supports
moduleResolution: "bundler"), but it duplicates and diverges from established project patterns:
- Does not extend
packages/typescript-config/nextjs.jsonlike other Next.js projects should- Uses ES2017 target instead of the project standard ES2022
- Excludes security settings like
noUncheckedIndexedAccessfrom the shared base config- Sets
incremental: trueconflicting with project base config (false)Update the tsconfig to extend the shared config:
{ "extends": "../../packages/typescript-config/nextjs.json", "compilerOptions": { "paths": { "@/*": ["./src/*"] } }, "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx", ".next/types/**/*.ts"], "exclude": ["node_modules"] }platforms/registry/src/services/HealthCheckService.ts-25-28 (1)
25-28: Health check logic accepts error status codes as healthy.Line 27's
validateStatus: (status) => status < 500treats any response with a status code below 500 as healthy—including 4xx errors (400, 401, 403, 404, etc.). The method returnstruefor these responses, but they indicate the whois endpoint is missing, forbidden, or misconfigured. A 404 response means the endpoint doesn't exist; a 403 means access is forbidden—neither indicates a healthy service.For semantic correctness, only 2xx responses should indicate a healthy check:
const response = await axios.get(whoisUrl, { timeout: this.timeout, - validateStatus: (status) => status < 500, // Accept any status < 500 as "reachable" + validateStatus: (status) => status >= 200 && status < 300, // Accept 2xx as "healthy" });infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte-201-201 (1)
201-201: Remove or protect debug logging of sensitive cryptographic material.Logging
registryEntropyexposes cryptographic tokens in the browser console, which could be captured by browser extensions or debugging tools.Apply this diff to remove the debug log:
const registryEntropy = entropyRes.data.token; - console.log("Registry entropy:", registryEntropy);Alternatively, if debug logging is needed during development, guard it with an environment check:
const registryEntropy = entropyRes.data.token; - console.log("Registry entropy:", registryEntropy); + if (import.meta.env.DEV) { + console.log("Registry entropy:", registryEntropy); + }infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte-212-212 (1)
212-212: Remove or protect debug logging of provision response.Logging
provisionRes.datamay expose sensitive provisioning information (w3id, URI, etc.) in the browser console.Apply this diff to remove the debug log:
- console.log("Provision response:", provisionRes.data);Alternatively, guard it with an environment check for development-only logging:
- console.log("Provision response:", provisionRes.data); + if (import.meta.env.DEV) { + console.log("Provision response:", provisionRes.data); + }infrastructure/evault-core/src/core/db/db.service.ts-728-804 (1)
728-804: Migration copy operation lacks transactional guarantees.If the copy fails mid-way (e.g., after copying some envelopes but before all), the target database will be left in a partial state. Consider wrapping the entire operation in a Neo4j transaction or implementing cleanup on failure.
Consider using a transaction for atomicity:
async copyMetaEnvelopesToNewEvaultInstance( eName: string, targetDbService: DbService, ): Promise<number> { // ... validation ... + // Note: For full atomicity, consider using a transaction session + // const session = targetDbService.driver.session(); + // const tx = session.beginTransaction(); + // try { ... tx.commit(); } catch { tx.rollback(); throw; } + // Copy each meta-envelope to the target evault for (const metaEnvelope of metaEnvelopes) {At minimum, document this limitation in the method's JSDoc.
Committable suggestion skipped: line range outside the PR's diff.
platforms/emover-api/src/index.ts-29-36 (1)
29-36: Removecredentials: truefrom CORS configWith
origin: "*", settingcredentials: trueviolates the CORS specification—browsers reject responses that combine wildcard origins with credential mode. Since this API uses Bearer token authentication (JWT),credentials: trueis unnecessary. Either remove it:app.use( cors({ origin: "*", methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], allowedHeaders: ["Content-Type", "Authorization"], - credentials: true, }), );Or, if credentials are needed in the future, replace the wildcard with an explicit origin from an environment variable:
+const allowedOrigin = process.env.EMOVER_WEB_ORIGIN ?? "http://localhost:3000"; + app.use( cors({ - origin: "*", + origin: allowedOrigin, methods: ["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], allowedHeaders: ["Content-Type", "Authorization"], credentials: true, }), );platforms/emover-api/src/controllers/MigrationController.ts-110-138 (1)
110-138: SSE endpoint lacks authentication.
getSessionStatusdoesn't validate the caller, allowing anyone to subscribe to any migration's updates by guessing the session ID.platforms/emover-api/src/database/entities/Migration.ts-53-57 (1)
53-57: Use optional property syntax for nullable text columns.Same issue with
logsanderrorfields.@Column("text", { nullable: true }) - logs!: string; + logs?: string; @Column("text", { nullable: true }) - error!: string; + error?: string;platforms/emover-api/src/controllers/AuthController.ts-62-62 (1)
62-62: Remove or redact PII from logs.Logging
ename(user identifier) in plaintext may violate privacy/compliance requirements (GDPR, CCPA). Remove this log statement or redact sensitive fields before logging.- console.log(ename, session, appVersion); + console.log("Login attempt", { session, appVersion });platforms/emover-api/src/database/entities/Migration.ts-28-44 (1)
28-44: Use optional property syntax for nullable columns.Fields marked with
{ nullable: true }should use?instead of!to accurately reflect that they may beundefinedat runtime. Using!on nullable columns is misleading and can cause runtime errors.@Column({ nullable: true }) - oldEvaultId!: string; + oldEvaultId?: string; @Column({ nullable: true }) - newEvaultId!: string; + newEvaultId?: string; @Column({ nullable: true }) - eName!: string; + eName?: string; @Column({ nullable: true }) - oldEvaultUri!: string; + oldEvaultUri?: string; @Column({ nullable: true }) - newEvaultUri!: string; + newEvaultUri?: string; @Column({ nullable: true }) - provisionerUrl!: string; + provisionerUrl?: string;platforms/emover-api/src/controllers/MigrationController.ts-187-208 (1)
187-208: Missing authentication check ongetStatus.Unlike other endpoints,
getStatusdoesn't verifyreq.user. Any unauthenticated user can query any migration's status, logs, and errors by ID.getStatus = async (req: Request, res: Response) => { try { + if (!req.user) { + return res.status(401).json({ error: "Authentication required" }); + } + const { id } = req.params; const migration = await this.migrationService.getMigrationById(id); if (!migration) { return res.status(404).json({ error: "Migration not found" }); } + if (migration.userId !== req.user.id) { + return res.status(403).json({ error: "Unauthorized" }); + } + return res.json({platforms/emover-api/src/controllers/MigrationController.ts-174-174 (1)
174-174: BlockingprocessMigrationcall may cause HTTP timeout.
processMigrationperforms multiple async operations (provisioning, copying, verifying, etc.) which could take significant time. Awaiting it in the callback handler may cause the HTTP request to timeout.Consider running the migration asynchronously and responding immediately:
- await this.processMigration(migrationId); + // Start migration asynchronously - don't block the response + this.processMigration(migrationId).catch((error) => { + console.error(`Background migration ${migrationId} failed:`, error); + }); return res.status(200).json({ success: true, - message: "Migration started", + message: "Migration started in background", migrationId, });platforms/emover-api/src/controllers/AuthController.ts-10-17 (1)
10-17: Instance-scoped EventEmitter may cause events to be lost.Each
AuthControllerinstance creates its ownEventEmitter. If the SSE stream subscription (sseStream) and the login event emission (login) are handled by different instances (e.g., due to Express route handler instantiation patterns), events will not be delivered to SSE clients.Consider using a module-level shared
EventEmitteror a dedicated pub/sub mechanism (e.g., Redis pub/sub for multi-instance deployments).+import { EventEmitter } from "events"; + +// Shared emitter for cross-request communication +const sharedEventEmitter = new EventEmitter(); + export class AuthController { private userService: UserService; - private eventEmitter: EventEmitter; constructor() { this.userService = new UserService(); - this.eventEmitter = new EventEmitter(); }Then replace
this.eventEmitterwithsharedEventEmitterthroughout the class.Committable suggestion skipped: line range outside the PR's diff.
platforms/emover-api/src/services/MigrationService.ts-305-317 (1)
305-317: Data verification is not implemented - returns success without checking.The
verifyDataCopymethod logs "Verification successful" and returnstruewithout performing any actual verification. This could allow migrations to complete with missing or corrupted data.The comment on lines 305-308 acknowledges this. At minimum, add a TODO or implement basic count verification by querying the new evault. Would you like me to help implement actual verification logic?
platforms/emover-api/src/services/MigrationService.ts-227-228 (1)
227-228: Insecure default Neo4j credentials.Using
"neo4j"/"neo4j"as default credentials is a security risk. If environment variables are not set, production could inadvertently use weak credentials.Consider requiring explicit configuration:
- const neo4jUser = process.env.NEO4J_USER || "neo4j"; - const neo4jPassword = process.env.NEO4J_PASSWORD || "neo4j"; + const neo4jUser = process.env.NEO4J_USER; + const neo4jPassword = process.env.NEO4J_PASSWORD; + + if (!neo4jUser || !neo4jPassword) { + throw new Error("NEO4J_USER and NEO4J_PASSWORD environment variables are required"); + }platforms/emover-api/src/services/MigrationService.ts-528-536 (1)
528-536: Old evault deletion is not implemented but migration is marked as COMPLETED.The method logs "Old evault deleted successfully" and sets
status = MigrationStatus.COMPLETEDwithout actually deleting anything (lines 528-530 acknowledge this). This leaves orphaned evaults and could lead to data inconsistency or storage waste.Either implement the deletion or change the status to something like
PENDING_CLEANUPto indicate manual intervention is needed:// Delete old evault - this would need an endpoint on the provisioner or evault service // For now, we'll just log it - // In production, implement actual deletion + // TODO: In production, implement actual deletion via provisioner API - migration.logs += `[MIGRATION] Old evault ${oldEvaultId} deleted successfully\n`; - migration.status = MigrationStatus.COMPLETED; + migration.logs += `[MIGRATION] Old evault ${oldEvaultId} marked for cleanup (manual deletion required)\n`; + migration.status = MigrationStatus.COMPLETED; // Consider: PENDING_CLEANUP statusWould you like me to help design the cleanup mechanism?
platforms/emover-api/src/services/MigrationService.ts-135-137 (1)
135-137: Hardcoded demo verification code may pose security risk.The hardcoded UUID fallback for
verificationIdcould be exploited in production ifDEMO_VERIFICATION_CODEis unset. This could allow unauthorized provisioning.Consider requiring the environment variable and failing explicitly:
- verificationId: - process.env.DEMO_VERIFICATION_CODE || - "d66b7138-538a-465f-a6ce-f6985854c3f4", + verificationId: (() => { + const code = process.env.DEMO_VERIFICATION_CODE; + if (!code) { + throw new Error("DEMO_VERIFICATION_CODE environment variable is required"); + } + return code; + })(),
🟡 Minor comments (2)
platforms/emover/src/app/(app)/migrate/page.tsx-81-130 (1)
81-130: Polling continues after terminal states; unused dependency.
- The interval continues polling even after
completedorfailedstatus. Add early return or clear the interval on terminal states.routeris in the dependency array but unused in the effect body.useEffect(() => { if (!migrationId) return; + let stopped = false; const pollStatus = async () => { + if (stopped) return; try { const response = await apiClient.get(`/api/migration/status/${migrationId}`); const data = response.data; if (data.status) { setMigrationStatus(data.status as MigrationStatus); // ... existing log parsing ... // Check if migration is complete or failed if (data.status === "completed") { setIsActivated(true); setQrData(null); + stopped = true; } else if (data.status === "failed") { setError(data.error || "Migration failed"); + stopped = true; } } } catch (error) { console.error("Error polling migration status:", error); } }; pollStatus(); const interval = setInterval(pollStatus, 2000); - return () => clearInterval(interval); - }, [migrationId, router]); + return () => { + stopped = true; + clearInterval(interval); + }; + }, [migrationId]);platforms/emover-api/src/services/MigrationService.ts-80-88 (1)
80-88: Missing timeouts on external HTTP calls.The axios calls to
/whois,/entropy,/provision, and/resolvelack timeouts. If any external service becomes unresponsive, the migration will hang indefinitely.Add reasonable timeouts to all external calls:
try { const whoisResponse = await axios.get( new URL("/whois", migration.oldEvaultUri || "").toString(), { headers: { "X-ENAME": eName, }, + timeout: 30000, // 30 seconds }, );Apply similar timeouts to other axios calls throughout the service.
🧹 Nitpick comments (33)
platforms/registry/src/services/HealthCheckService.ts (1)
45-68: URI parsing doesn't support IPv6 addresses.The
parseUrimethod uses a simple split-by-colon approach (line 51), which doesn't handle IPv6 addresses (e.g.,[::1]:8080orhttp://[2001:db8::1]:8080). If IPv6 support is needed for your deployment, the parsing logic must be updated.Verify whether IPv6 support is required for your infrastructure. If not, this can be deferred. If needed, consider using the
URLAPI for robust URI parsing:private parseUri(uri: string): { ip: string | null; port: string | null } { try { const url = new URL(uri.startsWith('http') ? uri : `http://${uri}`); const ip = url.hostname; const port = url.port || '80'; if (this.isValidIp(ip) && this.isValidPort(port)) { return { ip, port }; } return { ip: null, port: null }; } catch (error) { console.error(`Error parsing URI ${uri}:`, error); return { ip: null, port: null }; } }However, note that the current
isValidIpregex only validates IPv4; IPv6 validation logic would also need to be added.platforms/emover-api/README.md (1)
67-69: Specify language for fenced code block.The code block should have a language identifier for proper syntax highlighting.
Apply this diff:
-``` +```bash npm run migration:run</blockquote></details> <details> <summary>platforms/emover/README.md (1)</summary><blockquote> `16-16`: **Format URL consistently.** The bare URL could be formatted for better readability. Apply this diff: ```diff -- `NEXT_PUBLIC_EMOVER_BASE_URL` - Backend API base URL (default: http://localhost:4003) +- `NEXT_PUBLIC_EMOVER_BASE_URL` - Backend API base URL (default: `http://localhost:4003`)platforms/emover-api/tsconfig.json (1)
16-16: Consider using definite assignment assertions instead of disabling strictPropertyInitialization.While
strictPropertyInitialization: falseis common with TypeORM entities, it weakens type safety across the entire project. Consider keeping strict initialization enabled and using definite assignment assertions (!) on entity properties that are initialized by TypeORM decorators.Example:
@Column() name!: string; // definite assignment assertionscripts/get-eid-wallet-token.sh (1)
10-12: Consider adding a timeout to the curl command.The curl request lacks a timeout, which could cause the script to hang indefinitely if the registry is unresponsive.
Apply this diff:
-RESPONSE=$(curl -s -X POST "$REGISTRY_URL/platforms/certification" \ +RESPONSE=$(curl -s --max-time 10 -X POST "$REGISTRY_URL/platforms/certification" \ -H "Content-Type: application/json" \ -d '{"platform": "eid-wallet"}')platforms/emover/src/lib/apiClient.ts (1)
30-34: Consider preserving the intended destination URL.The hard redirect to
/logindoesn't preserve the URL the user was trying to access. Users will need to manually navigate back after re-authenticating.Consider preserving the current path:
if (error.response?.status === 401) { localStorage.removeItem("emover_token"); localStorage.removeItem("emover_user_id"); - window.location.href = "/login"; + const returnUrl = encodeURIComponent(window.location.pathname + window.location.search); + window.location.href = `/login?returnUrl=${returnUrl}`; }Then update your login page to redirect to
returnUrlafter successful authentication.platforms/emover-api/src/utils/version.ts (1)
3-16: Add input validation for version strings.The function doesn't validate version format before parsing. Non-numeric version parts will produce
NaN, leading to incorrect comparisons. If user-supplied version strings reach this function, malformed input could bypass version checks.Add validation:
export function isVersionValid(version: string, minVersion: string = MIN_REQUIRED_VERSION): boolean { + const versionRegex = /^\d+(\.\d+)*$/; + if (!versionRegex.test(version) || !versionRegex.test(minVersion)) { + return false; + } + const versionParts = version.split(".").map(Number); const minVersionParts = minVersion.split(".").map(Number);platforms/emover/src/lib/auth-context.tsx (1)
51-63: Consider adding a loading state during login.The login function doesn't set a loading state, which could be useful for UI feedback. Additionally, concurrent login calls could lead to race conditions.
Consider tracking login loading state:
export function AuthProvider({ children }: { children: React.ReactNode }) { const [user, setUser] = useState<User | null>(null); const [isLoading, setIsLoading] = useState(true); + const [isLoggingIn, setIsLoggingIn] = useState(false); const isAuthenticated = !!user; const login = async (ename: string) => { + if (isLoggingIn) return; // Prevent concurrent login calls + setIsLoggingIn(true); try { const response = await apiClient.post("/api/auth", { ename }); const { token, user: userData } = response.data; setAuthToken(token); setAuthId(userData.id); setUser(userData); } catch (error) { console.error("Login failed:", error); throw error; + } finally { + setIsLoggingIn(false); } };platforms/registry/src/index.ts (2)
62-74: Authentication middleware implementation looks correct.The
checkSharedSecretmiddleware properly validates the Bearer token format and compares against the environment variable. However, consider using a timing-safe comparison to prevent timing attacks.+import crypto from "node:crypto"; + const checkSharedSecret = async (request: any, reply: any) => { const authHeader = request.headers.authorization; if (!authHeader || !authHeader.startsWith("Bearer ")) { return reply .status(401) .send({ error: "Missing or invalid authorization header" }); } const secret = authHeader.split(" ")[1]; - if (secret !== process.env.REGISTRY_SHARED_SECRET) { + const expectedSecret = process.env.REGISTRY_SHARED_SECRET || ""; + if (!crypto.timingSafeEqual(Buffer.from(secret), Buffer.from(expectedSecret))) { return reply.status(401).send({ error: "Invalid shared secret" }); } };
110-111: Replaceconsole.logwithserver.log.debugfor consistent logging.Other endpoints use
server.log.errorandserver.log.info. Use the structured logger here for consistency.- console.log("Generating entropy"); + server.log.debug("Generating entropy");infrastructure/evault-core/src/core/db/db.service.ts (2)
806-832: User node copy failure is silently swallowed.The comment says "Don't fail the migration if User node copy fails," but this could leave the migration incomplete. Consider at least returning a warning in the response or logging at a higher severity level.
} catch (error) { - console.error(`[MIGRATION ERROR] Failed to copy User node:`, error); + console.warn(`[MIGRATION WARNING] Failed to copy User node (non-fatal):`, error); // Don't fail the migration if User node copy fails + // TODO: Consider returning this as a warning in the response }
834-872: Verification logic is thorough but duplicated.This verification is also performed in the HTTP handler (server.ts lines 680-719). Consider consolidating the verification logic to avoid maintenance burden and potential inconsistency.
platforms/emover-api/src/index.ts (1)
18-26: Consider starting the HTTP server only after DB initialization succeedsRight now the server starts listening immediately while the DB connection initializes in parallel, so early requests could hit before the data source is ready. You can simplify startup and avoid that race by only calling
app.listeninside the successful.then:-// Initialize database connection -AppDataSource.initialize() - .then(() => { - console.log("Database connection established"); - }) - .catch((error: unknown) => { - console.error("Error during initialization:", error); - process.exit(1); - }); - -// ... - -app.listen(port, () => { - console.log(`Emover API server running on port ${port}`); -}); +// Initialize database connection, then start server +AppDataSource.initialize() + .then(() => { + console.log("Database connection established"); + app.listen(port, () => { + console.log(`Emover API server running on port ${port}`); + }); + }) + .catch((error: unknown) => { + console.error("Error during initialization:", error); + process.exit(1); + });Also applies to: 70-72
platforms/emover/src/app/(app)/layout.tsx (1)
6-33: Auth gating is correct; consider centralizing it here and simplifying pagesThe redirect/useEffect and loading handling are sound for guarding the
(app)route group. SinceDashboardPage(and likely other children) also implement their ownisAuthenticated/redirect logic, you could simplify those pages and rely on this layout as the single source of truth for auth gating to avoid duplicaterouter.pushcalls and scattered checks.platforms/emover/src/app/(app)/page.tsx (2)
20-55: Auth + data loading logic works; consider relying on layout for gatingThe redirect effect and
if (!isAuthenticated) return null;checks are correct, but they duplicate what(app)/layout.tsxalready does. You could remove the auth redirect/early‑return here and assume that any route under(app)is already authenticated, letting this component focus purely on data fetching and UI. That would also avoid double redirects to/loginon state changes.
37-51: Optional: handle API errors more explicitly for better UXRight now, failures from
/api/evault/currentor/api/provisionersonly log to the console and leave the user with empty sections. Consider setting anerrorstate and surfacing a short message in the UI (and/or triggering logout on 401 if not already handled globally byapiClientinterceptors).platforms/emover-api/src/database/migrations/1764241399683-migration.ts (1)
3-16: Consider adding FK + indexes for evault_migrations and usersThe tables will work as‑is, but you’ll likely want some constraints/indexes for correctness and performance:
- Add a foreign key from
evault_migrations.userId→users.idto enforce referential integrity.- Add an index (and probably a unique constraint) on
users.enameif you intend eName to uniquely identify a user.- Add an index on
evault_migrations.userIdif you’ll query migrations by user frequently.Example (adapt names as needed):
public async up(queryRunner: QueryRunner): Promise<void> { @@ - await queryRunner.query(`CREATE TABLE "evault_migrations" ("id" uuid NOT NULL DEFAULT uuid_generate_v4(), "userId" uuid NOT NULL, "oldEvaultId" character varying, "newEvaultId" character varying, "eName" character varying, "oldEvaultUri" character varying, "newEvaultUri" character varying, "provisionerUrl" character varying, "status" "public"."evault_migrations_status_enum" NOT NULL DEFAULT 'initiated', "logs" text, "error" text, "createdAt" TIMESTAMP NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP NOT NULL DEFAULT now(), CONSTRAINT "PK_f32dd99f3ca413e7fbe0dbe418e" PRIMARY KEY ("id"))`); - await queryRunner.query(`CREATE TABLE "users" ("id" uuid NOT NULL DEFAULT uuid_generate_v4(), "ename" character varying, "name" character varying, "createdAt" TIMESTAMP NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP NOT NULL DEFAULT now(), CONSTRAINT "PK_a3ffb1c0c8416b9fc6f907b7433" PRIMARY KEY ("id"))`); + await queryRunner.query(`CREATE TABLE "evault_migrations" ("id" uuid NOT NULL DEFAULT uuid_generate_v4(), "userId" uuid NOT NULL, "oldEvaultId" character varying, "newEvaultId" character varying, "eName" character varying, "oldEvaultUri" character varying, "newEvaultUri" character varying, "provisionerUrl" character varying, "status" "public"."evault_migrations_status_enum" NOT NULL DEFAULT 'initiated', "logs" text, "error" text, "createdAt" TIMESTAMP NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP NOT NULL DEFAULT now(), CONSTRAINT "PK_f32dd99f3ca413e7fbe0dbe418e" PRIMARY KEY ("id"))`); + await queryRunner.query(`CREATE TABLE "users" ("id" uuid NOT NULL DEFAULT uuid_generate_v4(), "ename" character varying, "name" character varying, "createdAt" TIMESTAMP NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP NOT NULL DEFAULT now(), CONSTRAINT "PK_a3ffb1c0c8416b9fc6f907b7433" PRIMARY KEY ("id"))`); + await queryRunner.query(`CREATE UNIQUE INDEX "IDX_users_ename" ON "users" ("ename")`); + await queryRunner.query(`ALTER TABLE "evault_migrations" ADD CONSTRAINT "FK_evault_migrations_user" FOREIGN KEY ("userId") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE NO ACTION`); @@ public async down(queryRunner: QueryRunner): Promise<void> { + await queryRunner.query(`ALTER TABLE "evault_migrations" DROP CONSTRAINT "FK_evault_migrations_user"`); + await queryRunner.query(`DROP INDEX "IDX_users_ename"`); await queryRunner.query(`DROP TABLE "users"`); await queryRunner.query(`DROP TABLE "evault_migrations"`);platforms/emover/src/app/(auth)/login/page.tsx (2)
8-12: Clarify role ofuseAuth().loginvs manual token handling
loginfromuseAuthis pulled in and only referenced in theuseEffectdependency array; the actual login logic is handled viasetAuthToken,setAuthId, andwindow.location.href = "/".That’s fine if AuthContext bootstraps purely from localStorage on page load, but it’s a bit confusing:
- Either call
loginhere and centralize token/user persistence inside the auth context, e.g.login({ token: data.token, user: data.user }), letting it decide how to store things.- Or, if this flow is intentionally standalone, remove
useAuthentirely and droploginfrom the effect deps to reduce mental overhead.Right now it’s not obvious which source of truth should be preferred for auth state.
Also applies to: 35-71
26-29: Surface generic errors to the user for QR fetch / SSE failuresOn QR fetch failure and SSE
onerror, you onlyconsole.errorand/or close the EventSource. From the user’s perspective the page can end up in a non‑functional state with no feedback.Consider setting
errorMessagein these paths so the UI shows a retry hint:useEffect(() => { const fetchQRCode = async () => { try { @@ } catch (error) { console.error("Failed to fetch QR code:", error); - setIsLoading(false); + setIsLoading(false); + setErrorMessage( + "Unable to load login QR code. Please refresh the page or try again later.", + ); } }; @@ eventSource.onerror = () => { + setErrorMessage( + "Connection lost while waiting for login. Please refresh the page and try again.", + ); eventSource.close(); };This keeps the flow more robust in the face of network/registry issues.
Also applies to: 66-68, 73-80, 90-100
platforms/emover-api/src/middleware/auth.ts (1)
6-45: Confirm “optional auth + explicit guard” pattern matches route usage
authMiddlewarecurrently behaves as:
- No
Authorizationor non‑Bearer header →next()withreq.userunset.- Invalid token / unknown user → 401.
- Valid token → populates
req.userthennext().That’s a solid pattern if every route that requires authentication also adds
authGuard(or performs its ownreq.usercheck likeUserController.currentUser). If you ever mount onlyauthMiddlewareon a route assuming it enforces auth, missing headers will slip through as unauthenticated requests.Might be worth documenting this contract where you wire routes, or introducing a stricter variant (e.g.
strictAuthMiddleware) if you need “always 401 on missing auth” behavior for some endpoints.platforms/emover-api/src/services/UserService.ts (1)
4-29: Normalizeenameon create to match lookup semantics
findByEnamenormalizesenameto always consider both@fooandfoo, butcreateUserstores whatever string is passed in. Over time this can leave the DB with mixed formats.To keep data consistent and simplify reasoning, consider normalizing on write as well:
async createUser(ename: string, name?: string): Promise<User> { - const user = this.userRepository.create({ - ename, - name: name || ename, - }); + const normalizedEname = ename.startsWith("@") ? ename : `@${ename}`; + const user = this.userRepository.create({ + ename: normalizedEname, + name: name || normalizedEname, + }); return this.userRepository.save(user); }This way all stored users have a consistent
@‑prefixed handle, andfindByEnamebecomes more predictable.platforms/emover-api/src/database/entities/User.ts (1)
9-25: Align TypeScript nullability with database column definitions
enameandnameare declared as:@Column({ nullable: true }) ename!: string; @Column({ nullable: true }) name!: string;but the TypeScript types are non‑nullable
string. At runtime these can benull, which the type system won’t force you to handle.Either:
- Make them non‑nullable in the DB if you always require them, or
- Reflect DB reality in the types:
- @Column({ nullable: true }) - ename!: string; + @Column({ nullable: true }) + ename!: string | null; @@ - @Column({ nullable: true }) - name!: string; + @Column({ nullable: true }) + name!: string | null;This reduces the risk of unexpected
nullvalues propagating without checks.platforms/emover-api/src/controllers/EvaultInfoController.ts (1)
50-77: Harden provisioner URL parsing to avoid empty/invalid entries
getProvisionersreadsPROVISIONER_URLandPROVISIONER_URLS, splits on commas, and trims, but it can still emit empty strings if the env has trailing commas or accidental blanks:urls.push(...provisionerUrls.split(",").map((url) => url.trim()));Then
extractProviderFromUriwill receive"", and the API will return entries with emptyurl/name.Consider filtering and maybe validating URLs:
if (provisionerUrls) { - urls.push( - ...provisionerUrls.split(",").map((url) => url.trim()), - ); + urls.push( + ...provisionerUrls + .split(",") + .map((url) => url.trim()) + .filter((url) => url.length > 0), + ); } @@ - const provisioners = uniqueUrls.map((url) => ({ - url, - name: this.extractProviderFromUri(url), - description: `Provisioner at ${url}`, - })); + const provisioners = uniqueUrls.map((url) => ({ + url, + name: this.extractProviderFromUri(url), + description: `Provisioner at ${url}`, + }));Optionally, you could also drop entries where
extractProviderFromUrireturns the original string and that string is clearly not a valid URL.Also applies to: 84-91
platforms/emover-api/src/controllers/AuthController.ts (1)
89-95: Consider adding rate limiting for user creation.Auto-creating users on login could be exploited for enumeration or database flooding. Consider adding rate limiting or validation that the
enameoriginates from a trusted source.platforms/emover-api/src/database/entities/Migration.ts (1)
25-26: Consider adding a foreign key relation to User.
userIdis stored as a plain UUID column without a TypeORM relation. Adding a@ManyToOnerelation would provide referential integrity and enable eager/lazy loading.+ @ManyToOne(() => User) + @JoinColumn({ name: "userId" }) + user?: User; + @Column("uuid") userId!: string;platforms/emover/src/app/(app)/migrate/page.tsx (3)
100-109: Fragile log-based activation detection.Parsing logs with
includes("marked as active")is brittle—any log message change will break detection. Consider returning anisActivatedboolean from the status API instead.
132-159: SSE connection has no reconnection logic.On error, the EventSource closes without retry. For critical flows like migration signing, consider exponential backoff reconnection.
eventSource.onerror = () => { + console.warn("SSE connection error, reconnecting..."); eventSource.close(); + // Optionally reconnect after delay };
41-60: No loading indicator during initial migration initiation.Users see nothing while
initiateMigrationis in flight. Consider adding a loading state.platforms/emover-api/src/services/SigningService.ts (2)
14-14: In-memory session storage does not persist across restarts.Sessions stored in a
Mapwill be lost on server restart or in multi-instance deployments. For production, consider using Redis or database-backed session storage.
54-57: Store timeout reference to allow early cancellation.If a session is manually deleted or processed, the timeout continues running. Store the timeout ID in the session to allow cleanup.
+ const timeoutId = setTimeout(() => { - setTimeout(() => { this.sessions.delete(sessionId); }, 15 * 60 * 1000); + + // Store timeoutId if you need to cancel early + // (session as any).timeoutId = timeoutId;platforms/emover-api/src/controllers/MigrationController.ts (1)
4-4: Remove unused import.
uuidv4is imported but never used in this file.-import { v4 as uuidv4 } from "uuid";platforms/emover-api/src/services/MigrationService.ts (2)
8-17: Unused class property and potential initialization order issue.
evaultBaseUri(line 16-17) is declared but never used in the class.migrationRepositoryis initialized at class property level, which requiresAppDataSourceto be initialized before anyMigrationServiceinstance is created. If the data source isn't ready, this will throw.Consider removing the unused property and deferring repository access:
export class MigrationService extends EventEmitter { - private migrationRepository = AppDataSource.getRepository(Migration); + private get migrationRepository() { + return AppDataSource.getRepository(Migration); + } private registryUrl: string; - private evaultBaseUri: string; constructor() { super(); this.registryUrl = process.env.PUBLIC_REGISTRY_URL || "http://localhost:4321"; - this.evaultBaseUri = - process.env.EVAULT_BASE_URI || "http://localhost:4000"; }
372-395: Silently swallowing deletion errors may leave orphaned registry entries.If the deletion of the provisioner-created registry entry fails (lines 388-395), the migration continues successfully but leaves a potentially conflicting entry. This could cause issues if the w3id is reused.
Consider tracking this as a warning in the migration record for manual cleanup, or implementing a retry mechanism:
} catch (error) { - // Log but don't fail if deletion fails (entry might not exist or already deleted) console.warn( `[MIGRATION] Failed to delete provisioner registry entry:`, error, ); - migration.logs += `[MIGRATION] Warning: Could not delete provisioner registry entry for ${newW3id}\n`; + migration.logs += `[MIGRATION] Warning: Could not delete provisioner registry entry for ${newW3id}. Manual cleanup may be required.\n`; + // Consider: store this in a separate field for easier tracking of cleanup tasks }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
infrastructure/eid-wallet/src-tauri/gen/android/.idea/codeStyles/Project.xmlis excluded by!**/gen/**infrastructure/eid-wallet/src-tauri/gen/android/.idea/codeStyles/codeStyleConfig.xmlis excluded by!**/gen/**infrastructure/eid-wallet/src-tauri/gen/android/.idea/kotlinc.xmlis excluded by!**/gen/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
.env.example(1 hunks)README.md(2 hunks)db/init-multiple-databases.sh(1 hunks)dev-docker-compose.yaml(2 hunks)infrastructure/eid-wallet/src/lib/global/controllers/evault.ts(2 hunks)infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte(2 hunks)infrastructure/evault-core/src/core/db/db.service.ts(20 hunks)infrastructure/evault-core/src/core/http/server.ts(3 hunks)platforms/emover-api/README.md(1 hunks)platforms/emover-api/package.json(1 hunks)platforms/emover-api/src/controllers/AuthController.ts(1 hunks)platforms/emover-api/src/controllers/EvaultInfoController.ts(1 hunks)platforms/emover-api/src/controllers/MigrationController.ts(1 hunks)platforms/emover-api/src/controllers/UserController.ts(1 hunks)platforms/emover-api/src/database/data-source.ts(1 hunks)platforms/emover-api/src/database/entities/Migration.ts(1 hunks)platforms/emover-api/src/database/entities/User.ts(1 hunks)platforms/emover-api/src/database/migrations/1764241399683-migration.ts(1 hunks)platforms/emover-api/src/index.ts(1 hunks)platforms/emover-api/src/middleware/auth.ts(1 hunks)platforms/emover-api/src/services/MigrationService.ts(1 hunks)platforms/emover-api/src/services/SigningService.ts(1 hunks)platforms/emover-api/src/services/UserService.ts(1 hunks)platforms/emover-api/src/types/express.d.ts(1 hunks)platforms/emover-api/src/utils/jwt.ts(1 hunks)platforms/emover-api/src/utils/version.ts(1 hunks)platforms/emover-api/tsconfig.json(1 hunks)platforms/emover/README.md(1 hunks)platforms/emover/next-env.d.ts(1 hunks)platforms/emover/next.config.ts(1 hunks)platforms/emover/package.json(1 hunks)platforms/emover/postcss.config.mjs(1 hunks)platforms/emover/src/app/(app)/layout.tsx(1 hunks)platforms/emover/src/app/(app)/migrate/page.tsx(1 hunks)platforms/emover/src/app/(app)/page.tsx(1 hunks)platforms/emover/src/app/(auth)/login/page.tsx(1 hunks)platforms/emover/src/app/globals.css(1 hunks)platforms/emover/src/app/layout.tsx(1 hunks)platforms/emover/src/lib/apiClient.ts(1 hunks)platforms/emover/src/lib/auth-context.tsx(1 hunks)platforms/emover/src/lib/authUtils.ts(1 hunks)platforms/emover/tailwind.config.ts(1 hunks)platforms/emover/tsconfig.json(1 hunks)platforms/registry/src/index.ts(6 hunks)platforms/registry/src/services/HealthCheckService.ts(4 hunks)platforms/registry/src/services/VaultService.ts(1 hunks)scripts/get-eid-wallet-token.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-05-08T08:47:11.295Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/app.css:1-1
Timestamp: 2025-05-08T08:47:11.295Z
Learning: In Tailwind CSS v4, the traditional directive system has been removed. The `tailwind base` directive should be replaced with `import "tailwindcss/preflight";` for just base styles, or `import "tailwindcss";` to include all layers (preflight, components, and utilities).
Applied to files:
platforms/emover/src/app/globals.css
📚 Learning: 2025-05-08T08:47:11.295Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/app.css:1-1
Timestamp: 2025-05-08T08:47:11.295Z
Learning: In Tailwind CSS v4, the traditional `tailwind base` directive is no longer available and should be replaced with `import "tailwindcss/preflight"` instead. The directive system has been changed in v4 to use direct imports.
Applied to files:
platforms/emover/src/app/globals.css
📚 Learning: 2025-08-08T04:15:35.098Z
Learnt from: sosweetham
Repo: MetaState-Prototype-Project/prototype PR: 279
File: infrastructure/control-panel/src/app.css:0-0
Timestamp: 2025-08-08T04:15:35.098Z
Learning: The MetaState-Prototype-Project uses Tailwind CSS v3, where the theme directive is valid and supported. The theme block should not be changed to :root or layer base in this project.
Applied to files:
platforms/emover/src/app/globals.css
📚 Learning: 2025-11-13T10:34:52.527Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 415
File: infrastructure/eid-wallet/src/env.d.ts:8-8
Timestamp: 2025-11-13T10:34:52.527Z
Learning: In infrastructure/eid-wallet, PUBLIC_PLATFORM_URL should not be added to .env.example or configured as a static environment variable. The platform URL is extracted dynamically through URI parsing according to the protocol specification, and all fallbacks for platform URL are being removed.
Applied to files:
scripts/get-eid-wallet-token.sh.env.example
🧬 Code graph analysis (15)
platforms/emover/src/app/layout.tsx (1)
platforms/emover/src/lib/auth-context.tsx (1)
AuthProvider(25-75)
platforms/emover/tailwind.config.ts (1)
platforms/emover/postcss.config.mjs (1)
config(2-6)
platforms/emover-api/src/utils/jwt.ts (1)
infrastructure/evault-core/generate-test-token.js (1)
token(39-50)
platforms/emover/src/app/(app)/page.tsx (3)
platforms/emover/src/lib/auth-context.tsx (1)
useAuth(77-83)platforms/group-charter-manager/src/lib/authUtils.ts (1)
isAuthenticated(5-10)platforms/emover/src/lib/apiClient.ts (1)
apiClient(5-10)
platforms/emover-api/src/database/migrations/1764241399683-migration.ts (1)
platforms/evoting-api/src/database/migrations/1754593951325-migration.ts (1)
Migration1754593951325(3-32)
platforms/emover/src/app/(auth)/login/page.tsx (3)
platforms/emover/src/lib/auth-context.tsx (1)
useAuth(77-83)platforms/emover/src/lib/apiClient.ts (1)
apiClient(5-10)platforms/emover/src/lib/authUtils.ts (2)
setAuthToken(6-9)setAuthId(16-19)
platforms/emover-api/src/controllers/AuthController.ts (3)
platforms/emover-api/src/services/UserService.ts (1)
UserService(4-30)platforms/emover-api/src/utils/version.ts (1)
isVersionValid(3-16)platforms/emover-api/src/utils/jwt.ts (1)
signToken(5-7)
platforms/emover-api/src/database/entities/Migration.ts (1)
platforms/emover-api/src/database/entities/User.ts (1)
Entity(9-25)
platforms/emover-api/src/middleware/auth.ts (3)
infrastructure/evault-core/generate-test-token.js (1)
token(39-50)platforms/emover-api/src/utils/jwt.ts (1)
verifyToken(9-15)platforms/emover-api/src/database/data-source.ts (1)
AppDataSource(25-25)
infrastructure/evault-core/src/core/db/db.service.ts (2)
infrastructure/evault-core/src/core/db/schema.ts (2)
serializeValue(58-74)deserializeValue(76-85)infrastructure/evault-core/src/core/db/types.ts (3)
MetaEnvelopeResult(26-36)Envelope(14-19)GetAllEnvelopesResult(62-62)
platforms/emover-api/src/services/UserService.ts (1)
platforms/emover-api/src/database/data-source.ts (1)
AppDataSource(25-25)
platforms/emover-api/src/database/entities/User.ts (1)
platforms/emover-api/src/database/entities/Migration.ts (1)
Entity(20-64)
platforms/emover/src/lib/auth-context.tsx (2)
platforms/emover/src/lib/authUtils.ts (5)
getAuthToken(1-4)getAuthId(11-14)clearAuth(21-25)setAuthToken(6-9)setAuthId(16-19)platforms/emover/src/lib/apiClient.ts (1)
apiClient(5-10)
platforms/registry/src/index.ts (1)
infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (2)
vault(421-513)vault(515-528)
platforms/emover-api/src/services/MigrationService.ts (1)
platforms/emover-api/src/database/data-source.ts (1)
AppDataSource(25-25)
🪛 markdownlint-cli2 (0.18.1)
platforms/emover/README.md
16-16: Bare URL used
(MD034, no-bare-urls)
platforms/emover-api/README.md
72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: test-web3-adapter-integration
- GitHub Check: build
| // Emover endpoint - Copy metaEnvelopes to new evault instance | ||
| server.post<{ | ||
| Body: { | ||
| eName: string; | ||
| targetNeo4jUri: string; | ||
| targetNeo4jUser: string; | ||
| targetNeo4jPassword: string; | ||
| }; | ||
| }>( | ||
| "/emover", | ||
| { | ||
| schema: { | ||
| tags: ["migration"], | ||
| description: | ||
| "Copy all metaEnvelopes for an eName to a new evault instance", | ||
| body: { | ||
| type: "object", | ||
| required: [ | ||
| "eName", | ||
| "targetNeo4jUri", | ||
| "targetNeo4jUser", | ||
| "targetNeo4jPassword", | ||
| ], | ||
| properties: { | ||
| eName: { type: "string" }, | ||
| targetNeo4jUri: { type: "string" }, | ||
| targetNeo4jUser: { type: "string" }, | ||
| targetNeo4jPassword: { type: "string" }, | ||
| }, | ||
| }, | ||
| response: { | ||
| 200: { | ||
| type: "object", | ||
| properties: { | ||
| success: { type: "boolean" }, | ||
| count: { type: "number" }, | ||
| message: { type: "string" }, | ||
| }, | ||
| }, | ||
| 400: { | ||
| type: "object", | ||
| properties: { | ||
| error: { type: "string" }, | ||
| }, | ||
| }, | ||
| 500: { | ||
| type: "object", | ||
| properties: { | ||
| error: { type: "string" }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| async (request: TypedRequest<ProvisionRequest>, reply: TypedReply) => { | ||
| const result = await provisioningService.provisionEVault(request.body); | ||
| if (!result.success) { | ||
| return reply.status(500).send(result); | ||
| } | ||
| return result; | ||
| } | ||
| async ( | ||
| request: TypedRequest<{ | ||
| eName: string; | ||
| targetNeo4jUri: string; | ||
| targetNeo4jUser: string; | ||
| targetNeo4jPassword: string; | ||
| }>, | ||
| reply: TypedReply, | ||
| ) => { | ||
| const { | ||
| eName, | ||
| targetNeo4jUri, | ||
| targetNeo4jUser, | ||
| targetNeo4jPassword, | ||
| } = request.body; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: /emover endpoint accepts Neo4j credentials without authentication.
This endpoint accepts target database credentials (targetNeo4jUser, targetNeo4jPassword) in the request body but has no authentication guard. Any caller can trigger data migration and potentially exfiltrate data to an attacker-controlled Neo4j instance.
Add authentication similar to other sensitive endpoints:
server.post<{
Body: {
eName: string;
targetNeo4jUri: string;
targetNeo4jUser: string;
targetNeo4jPassword: string;
};
}>(
"/emover",
{
+ preHandler: async (request, reply) => {
+ const authHeader = request.headers.authorization || request.headers.Authorization;
+ const tokenPayload = await validateToken(
+ typeof authHeader === "string" ? authHeader : null,
+ );
+ if (!tokenPayload) {
+ return reply.status(401).send({ error: "Authentication required" });
+ }
+ },
schema: {| "dependencies": { | ||
| "axios": "^1.6.7", | ||
| "cors": "^2.8.5", | ||
| "dotenv": "^16.4.5", | ||
| "express": "^4.18.2", | ||
| "jsonwebtoken": "^9.0.2", | ||
| "pg": "^8.11.3", | ||
| "reflect-metadata": "^0.2.1", | ||
| "typeorm": "^0.3.24", | ||
| "uuid": "^9.0.1" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
axios 1.6.7 security vulnerabilities CVE December 2025
💡 Result:
Short answer: Yes — axios 1.6.7 is affected by multiple publicly disclosed vulnerabilities (SSRF and DoS) published in 2024–2025. Recommended remediation is to upgrade to a patched release (at minimum to the axios 1.8.2+ / 1.11.0+ / 1.12.0+ line depending on the issue — best to upgrade to the latest axios stable release). Sources:
- CVE-2025-27152 — SSRF / credential leakage: affects >=1.0.0, <1.8.2 (so 1.6.7 is vulnerable); patched in 1.8.2. [1][2]
- CVE-2024-39338 — SSRF (path-relative / protocol-relative URL handling): affects >1.3.2, <1.7.4 (so 1.6.7 is vulnerable). [3]
- CVE-2025-58754 — Denial‑of‑Service via large data: URI (memory exhaustion): affects versions before the 1.11/1.12 line (so 1.6.7 is vulnerable); patched in the 1.11.0/1.12.0+ releases. [4][5]
- CVE-2024-57965 — origin validation / related issue affecting versions before 1.7.8 (1.6.7 included). [6]
Recommended actions:
- Upgrade axios to the latest patched release (or at least to versions that fix each CVE: 1.8.2+ for CVE-2025-27152; 1.11.0+/1.12.0+ for the data: URI DoS; and 1.7.8+ for the origin check).
- If immediate upgrade isn’t possible, block or validate untrusted absolute/protocol‑relative URLs and disallow processing of untrusted data: URIs; add size checks before decoding; and apply network-level controls to reduce SSRF risk.
Sources:
[1] GitHub Advisory — axios SSRF (GHSA / CVE-2025-27152).
[2] NVD entry for CVE-2025-27152.
[3] Vendor/advisory records for CVE-2024-39338 (axios path/protocol-relative SSRF).
[4] NVD entry for CVE-2025-58754 (data: URI DoS).
[5] Multiple advisories / vendor bulletins about CVE-2025-58754 (patch references).
[6] IBM security bulletin re: CVE-2024-57965.
If you want, I can fetch exact release numbers and links for the latest axios stable release on npm/GitHub and provide commands to update and test.
🌐 Web query:
jsonwebtoken 9.0.2 security vulnerabilities CVE December 2025
💡 Result:
Short answer: As of December 3, 2025, jsonwebtoken 9.0.2 has no public CVEs affecting that version — the CVEs published in 2022 (notably CVE-2022-23529, CVE-2022-23539, CVE-2022-23540 and CVE-2022-23541) applied to versions ≤ 8.5.1 and were fixed by upgrading to 9.0.0+. [1][2][3][4]
Sources:
- Auth0 security bulletin (upgrade to 9.0.0+ to address CVE-2022-23539/23540/23541). [1]
- NVD / CVE entries describing CVE-2022-23540 and related issues. [2]
- OSV / CVE-2022-23541 record. [3]
- Snyk package page showing no known direct vulnerabilities in 9.0.2. [4]
🌐 Web query:
express 4.18.2 security vulnerabilities CVE December 2025
💡 Result:
Short answer: Yes — Express 4.18.2 is known to contain several disclosed security issues (open-redirect / XSS and transitive dependency problems). Key tracked CVEs include CVE-2024-29041 (open redirect), CVE-2024-43796 (XSS via redirect), plus multiple transitive issues from path-to-regexp and cookie packages (e.g., CVE-2024-45296, CVE-2024-52798, CVE-2024-47764). Remediation: upgrade Express (4.18.3/4.19.x/4.20.0/4.21+ as appropriate) or move to Express 5.x; update vulnerable transitive deps. [1][2][3]
Sources:
- GitHub security report listing CVEs affecting express‑4.18.2 and affected transitive deps. [1]
- IBM security bulletin calling out CVE‑2024‑29041 in express‑4.18.2. [2]
- Snyk vulnerability page for [email protected] (Open Redirect, XSS details + recommended upgrade versions). [3]
Would you like explicit mitigation steps (exact versions to update to and package.json examples)?
🌐 Web query:
latest stable versions axios jsonwebtoken express December 2025
💡 Result:
Latest stable versions (as of Dec 3, 2025):
- axios: 1.13.2. [1]
- express: 5.1.0. [2]
- jsonwebtoken: 9.0.2. [3]
Sources:
[1] axios releases (npm release listing).
[2] Express blog — "v5.1.0: Now the Default on npm" (Mar 31, 2025).
[3] jsonwebtoken package on npm (version 9.0.2).
Critical: Upgrade vulnerable dependencies immediately.
Three security issues require urgent action:
-
axios 1.6.7: Affected by multiple disclosed CVEs (CVE-2025-27152 SSRF/credential leakage, CVE-2024-39338 path-relative SSRF, CVE-2025-58754 DoS via data: URI, CVE-2024-57965 origin validation). Upgrade to at least 1.8.2+, preferably the latest 1.13.2.
-
express 4.18.2: Vulnerable to CVE-2024-29041 (open redirect) and CVE-2024-43796 (XSS via redirect), plus transitive dependency vulnerabilities (CVE-2024-45296, CVE-2024-52798, CVE-2024-47764). Upgrade to 5.1.0 or at minimum apply patches to 4.18.x line.
-
jsonwebtoken 9.0.2: No known vulnerabilities in this version (prior CVEs in ≤8.5.1 were fixed in 9.0.0+). Current.
| // Verify signature (simplified - in production, use proper signature verification) | ||
| // For now, we'll just check that signature exists | ||
| if (!signature) { | ||
| return { success: false, error: "Invalid signature" }; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Signature verification is not implemented.
The code only checks for signature presence, not validity. This allows any non-empty signature to authorize a migration, completely bypassing authentication.
Add proper cryptographic signature verification before production deployment:
- // Verify signature (simplified - in production, use proper signature verification)
- // For now, we'll just check that signature exists
- if (!signature) {
- return { success: false, error: "Invalid signature" };
- }
+ // TODO: Implement proper signature verification
+ // Example: verify signature against user's public key
+ const isValid = await this.verifySignature(message, signature, w3id);
+ if (!isValid) {
+ return { success: false, error: "Invalid signature" };
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In platforms/emover-api/src/services/SigningService.ts around lines 79 to 83,
the current code only checks that a signature string exists and does not verify
it cryptographically; replace this placeholder with real signature verification:
parse the signed payload you expect, load the trusted public key(s) from
configuration/env, use the appropriate crypto verifier for the algorithm used by
senders (e.g., RSA/ECDSA/Ed25519) to verify the payload bytes against the
provided signature, ensure verification fails returns { success: false, error:
"Invalid signature" } and succeeds returns success, perform constant-time
comparisons where applicable, and additionally validate freshness
(nonce/timestamp) to prevent replay attacks; make sure to surface errors for
logging while not leaking secrets.
| import jwt from "jsonwebtoken"; | ||
|
|
||
| const JWT_SECRET = process.env.JWT_SECRET || "your-secret-key-change-in-production"; | ||
|
|
||
| export function signToken(payload: { userId: string }): string { | ||
| return jwt.sign(payload, JWT_SECRET, { expiresIn: "7d" }); | ||
| } | ||
|
|
||
| export function verifyToken(token: string): { userId: string } | null { | ||
| try { | ||
| return jwt.verify(token, JWT_SECRET) as { userId: string }; | ||
| } catch (error) { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Search for JWT-related code in the emover-api service
rg -t ts "jwt|JWT_SECRET" platforms/emover-api/src --no-heading -A 2 -B 2Repository: MetaState-Prototype-Project/prototype
Length of output: 1894
🏁 Script executed:
# Check for environment configuration files
find platforms/emover-api -type f \( -name "*.env*" -o -name "*.config.*" -o -name "*.yml" -o -name "*.yaml" \) | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Look for other JWT imports or usage patterns
rg "import.*jwt|from.*jwt" platforms/emover-api/src -t tsRepository: MetaState-Prototype-Project/prototype
Length of output: 335
🏁 Script executed:
# Check if JWT_SECRET is mentioned in any documentation or setup files
rg "JWT_SECRET" platforms/emover-api --type ts --type md --type json --type yamlRepository: MetaState-Prototype-Project/prototype
Length of output: 487
🏁 Script executed:
# Check AuthController to see signToken usage
cat platforms/emover-api/src/controllers/AuthController.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 3718
🏁 Script executed:
# Check auth middleware to see verifyToken usage
cat platforms/emover-api/src/middleware/auth.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 1428
Remove hard-coded JWT_SECRET fallback and bind explicit algorithms
The current implementation allows JWT_SECRET to fall back to a static string if the environment variable is missing, creating a security risk if the secret isn't properly configured in production. Additionally, sign and verify rely on library defaults for algorithms rather than explicitly specifying them, which can lead to algorithm confusion attacks.
Remove the fallback and enforce an explicit algorithm to fail fast at startup if the secret isn't provided:
import jwt from "jsonwebtoken";
-const JWT_SECRET = process.env.JWT_SECRET || "your-secret-key-change-in-production";
+const JWT_SECRET = process.env.JWT_SECRET;
+if (!JWT_SECRET) {
+ throw new Error("JWT_SECRET must be set");
+}
export function signToken(payload: { userId: string }): string {
- return jwt.sign(payload, JWT_SECRET, { expiresIn: "7d" });
+ return jwt.sign(payload, JWT_SECRET, {
+ expiresIn: "7d",
+ algorithm: "HS256",
+ });
}
export function verifyToken(token: string): { userId: string } | null {
try {
- return jwt.verify(token, JWT_SECRET) as { userId: string };
+ return jwt.verify(token, JWT_SECRET, {
+ algorithms: ["HS256"],
+ }) as { userId: string };
} catch (error) {
return null;
}
}This ensures the service fails immediately if the required secret is missing and prevents algorithm confusion vulnerabilities.
🤖 Prompt for AI Agents
In platforms/emover-api/src/utils/jwt.ts around lines 1 to 15, remove the
hard-coded JWT_SECRET fallback and fail fast if process.env.JWT_SECRET is not
provided; replace the optional fallback by throwing an error (or otherwise
exiting) during module initialization when the secret is missing so the app
cannot start with a default insecure key, and explicitly specify a strong
algorithm (e.g., HS256) in both jwt.sign and jwt.verify calls to prevent
algorithm confusion attacks; keep the existing payload/return types and ensure
verify still returns null on verification errors.
Description of change
makes move from one evault to other possible (intra provisioner), and lays groundwork for inter provisioner evault moves possible
branches off from #466
Issue Number
n/a
Type of change
How the change has been tested
manual
Change checklist